DSL Items Parser: Fix incorrect parsing of keywords encountered in the wrong context - #4928

@jimtng many thanks for picking this up. It seems that the tests, although running, are now failing, but you are obviously on the right track. I have no knowledge of xtext so can’t really help any more with the details..

This passed now. However I've updated the original post with some further explanation

@jimtng I think the ^ prefix is a horrible hack, and IMHO not a solution. The parser should break on new lines, and if the word Switch is at the start of the line it should be interpreted as an item type; and if anywhere else it should be interpreted as a string (label or tag name). I think a simple regex could easily identify the two cases.

Note that if this PR is not merged, the code generating DSL format for item has to be fixed to add the character `^ when expected when generating tags.
https://github.com/openhab/openhab-core/blob/main/bundles/org.openhab.core.model.item/src/org/openhab/core/model/item/internal/fileconverter/DslItemFileConverter.java#L146

^ and we would probably need for safety sake to fix all item getTag(s) and setTag(s) methods to eliminate any stray carets. As I say, I think it is a horrible hack, and I strongly argue to solve this properly. We all know the proper syntax since years, and we should not allow a compromise in order to avoid a fix.

If the current parser really cannot distinguish between Switch at the start of a line and Switch anywhere else, the we should implement a regex pre-parser to do that in an inter- phase between loading the file and passing the stream to the existing parser.

But when the user writes ^Switch, I guess that the XText parser automatically removes the ^. So the resulting tag is Switch as we expect.

Maybe. Who knows. My argument is to make the code accept the syntax that every user knows since OH v1.0 and not introduce some queer kludge to avoid fixing a bug in the code that will confuse everybody.

Apparently noone was annoyed by this "bug" since several years ;)

I just checked how I myself solved this problem in my own file. I did not use ^ character at all but all my tags are between double quotes like for example: [ "Switch" ]

IMHO, as string are accepted in addition to ID by the parser, it is fine and there is nothing to fix. If the user wants to use a tag that is a special keyword, he just has to surround it with double-quotes.

But @jimtng proposal is also an improvement as it allows a syntax that was not possible before, so why not.
I see that more as an enhancement than as a bug fix.

Can either of you explain why the parser is not capable to detect the difference between Switch at the beginning of a line and Switch elsewhere? It cannot be so difficult.

I also checked for an item what is the generated code from Main UI and everything is OK, this is not a string that is used but the escape character:

Switch bypassed "ignorée" <switch> [^Switch] { channel="xxxxxx" }

So nothing to change/fix in the DSL generator.

I would suggest to not start a big change with risk while this is something that everyone using .items already knows how to handle. Maybe something could be added in our documentation to help someone who could encounter that error for the first time.

this is something that everyone using .items already knows how to handle

I don't agree.

For tags, including generating DSL syntax, I'd suggest just using quoted strings instead of caret or bare.

I tried finding a syntax or solution but haven't found one that's simple/straight forward.

I also checked for an item what is the generated code from Main UI and everything is OK, this is not a string that is used but the escape character:

Switch bypassed "ignorée" <switch> [^Switch] { channel="xxxxxx" }

So nothing to change/fix in the DSL generator.

indeed, that's good to know!

I tried creating an item in the UI:

• Name: Switch
• Tag: Switch

That worked and generated the correct DSL: String ^Switch "Switch" [^Switch] -> DSL Parser is happy to load it

Then I set the icon to Switch, DSL generated this: String ^Switch "Switch" <Switch> [^Switch] -> DSL Parser is not happy.
It needs to be manually adjusted to String ^Switch1 "Switch" <^Switch> [^Switch] to make DSL parser happy

Marking this PR as Draft. I'm not happy with the current solution.

@jimtng : Kai just merged PR #4793 (and that is a very good news for our pending stuff in UI) and that means you will have to adjust your PR carefully.
I will check it again after you solve the conflicts.

Build failed, you need to apply spotless in /org/openhab/core/model/item

This pull request has been mentioned on openHAB Community. There might be relevant details there:

https://community.openhab.org/t/rules-dsl-in-5-1-is-now-unloading-rules-if-it-has-unreachable-expressions/168382/2